[SYCL] Add support to propagate compile flags to device backend compiler#8763
Conversation
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
…ort_for_compile_flag_propagate_try_2
| ; RUN: FileCheck %s -input-file=%t_0.ll --check-prefixes CHECK-M0-IR \ | ||
| ; RUN: --implicit-check-not kernel0 --implicit-check-not kernel1 | ||
| ; RUN: FileCheck %s -input-file=%t_1.ll --check-prefixes CHECK-M1-IR \ | ||
| ; RUN: FileCheck %s -input-file=%t_2.ll --check-prefixes CHECK-M1-IR \ |
There was a problem hiding this comment.
These test updates were triggered by the fact that the order of the entries in the .table file changed due to the addition of the new entry in the list optional kernel features. A better way to NOT depend on the order of the entries is required in the long run.
There was a problem hiding this comment.
FYI: that unstable ordering is going to be fixed in #8833. That PR will also allow to simplify your changes to device code split down to a single line of code
|
An E2E test is added here: intel/llvm-test-suite#1691 Thanks |
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
Hi @bader This PR is a major revamp. Adding this on top of #7373 might end up confusing new reviewers. Alexey Sachkov is the main reviewer so far and we did discuss this. Thanks |
| } else if (Plugin.getBackend() == backend::opencl) { | ||
| if (!CompileOpts.empty() && (optLevel == 0)) | ||
| CompileOpts += " "; | ||
| if (optLevel == 0) |
There was a problem hiding this comment.
so for opt > 0 we ignore it for ocl? should we at least warn?
There was a problem hiding this comment.
Added warning message for cases where backend does not provide optimization option. Thanks
| | Front-end option | L0 backend option | OpenCL backend option | | ||
| | ---------------- | ----------------- | --------------------- | | ||
| | -O0 | -ze-opt-disable | -cl-opt-disable | | ||
| | -O1 | -ze-opt-level=1 | /* no option */ | |
There was a problem hiding this comment.
i commented this somewhere else but silently ignoring it seems confusing for the user, i recommend a warning
There was a problem hiding this comment.
We should be really careful with adding such warning. The problem here is that we can't suggest anything to user to fix this. There is literally no equivalent for different optimization levels in OpenCL: you either pass no flags, which means compiler optimizes as it wants to; or you pass -cl-opt-disable, which mean no optimizations at all.
-O2 is a default level of optimizations and we definitely don't want to have warning with no (!) workaround emitted for almost all existing applications.
Unless we can suggest a way to fix (in user code/build script) and not just suppress the warning, it shouldn't be added, I believe.
However, it makes sense to add a section describing this behavior into our documentation somewhere
There was a problem hiding this comment.
Thanks for detailed input on this. I am currently letting the users know about this via a warning. ""Optimization level not propagated to backend"; Do you think this is something which might confuse the user?
Do you want me to add the information in the design doc here?
Thanks again.
There was a problem hiding this comment.
Thanks for detailed input on this. I am currently letting the users know about this via a warning. ""Optimization level not propagated to backend"; Do you think this is something which might confuse the user?
I think any warning should be somehow resolvable by doing something other than -Wno-this-particular-warning (i.e. suppressing it). One possible pain point is -Werror configured on customer side. Therefore, I would recommend not to add the warning at all.
Do you want me to add the information in the design doc here?
In my last sentence about documentation I mean some user-facing documentation. Just so that our customers are aware about implementation details and can build their expectations accordingly.
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
I don't think this failure is still there with your latest test. |
yes. I added this comment based on the previous run. Thanks |
intel#8763 Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
intel#8763 Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
intel#8763 Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
intel#8763 Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
intel#8763 Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
intel#8763 Signed-off-by: Jaime Arteaga <jaime.a.arteaga.molina@intel.com>
This change is a second attempt to add this support. An earlier attempt was here: #7373
In this change, following changes have been made:
Thanks